Improve RequiredPluginsInitializer.initialize#1711
Conversation
| setSystem(true); | ||
| } | ||
|
|
||
| public synchronized void initialize(IJavaProject project) { |
There was a problem hiding this comment.
Its more performant for what that matters; i.e., no bridge method to make it visible... It's in a private class so it doesn't matter either way...
|
I'd actually like to further improve this so that org.eclipse.pde.internal.core.RequiredPluginsInitializer.setupClasspath(IJavaProject) can operate on multiple projects and would make a single call to JavaCore.setClasspathContainer(IPath, IJavaProject[], IClasspathContainer[], IProgressMonitor). For reasons that I cannot explain, I cannot reproduce the problem with this implementation but can easily and consistently reproduce it with the existing implementation. Maybe the timing of multiple concurrent jobs setting the classpath in parallel versus this implementation that batches them and does the sequentially? I don't know. |
|
FYI, I'm continue to investigate all the strange behaviors that I see in JDT's handling of various resource deltas. |
|
With my further optimized implementation (not yet pushed this PR) here is what I observe... The method JavaReconciler.forceReconciling() appears to key updating the editor to remove the error markers. I have instrumented it like this:
That method is guarded by
Here is the output: So setting the classpath does result in the method being called, but it's happen before If I add a 5000ms scheduling delay, the order of events is reversed and the editor is updated properly: So the problem remains that JDT is doing expensive (block IO) things on the UI thread and is doing it too early (no jobs enabled). In addition JDT is also doing editor initialization on a background thread, not on a job that we could join. As a result, the editor is not in a state to respond properly to incoming events, ignoring them as if they never happened. On the PDE side of things it seems we can only provide hacks. |
|
|
||
| public synchronized void initialize(IJavaProject project) { | ||
| if (projects.add(project)) { | ||
| schedule(5000); |
There was a problem hiding this comment.
Maybe this should use a system property with a default of 5000 so that someone one a slow machine could maybe configure this differently?
There was a problem hiding this comment.
FYI, with the changes from eclipse-jdt/eclipse.jdt.ui#2127 the delay is not needed...
- Collect deferred projects to be processed by a single job. eclipse-pde#1667
| protected void setupClasspath(IJavaProject javaProject) throws JavaModelException { | ||
| IProject project = javaProject.getProject(); | ||
| // The first project to be built may initialize the PDE models, potentially long running, so allow cancellation | ||
| protected static void setupClasspath(IJavaProject... javaProjects) throws JavaModelException { |
There was a problem hiding this comment.
Is it really useful to change this method for the case we actually like to avoid? How many projects are there to be collected that its worth to optimize here?
There was a problem hiding this comment.
Yes, it can be quite a large number, 18 and 35 I've seen; all timing related of course. The overhead downstream is massive, spawning huge chains of events that if you step over it steps for a very long time. Given we are batching the projects it makes sense to me to batch the whole deal so that all the classpath updates happen as a single workspace operation that could kick off a single build. In fact there is so much overhead that when I added this optimization the problem at the JDT side of things came back because of the fewer events bubbling through the system.
3 participants
#1667